Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust width of highway=construction rendering #3580

Merged
merged 3 commits into from
Jan 3, 2019

Conversation

jeisenbe
Copy link
Collaborator

Fixes #2595

Changes proposed in this pull request:

  • Stop rendering undocumented construction=* types
  • Specifically render raceway, pedestrian, living_street, road as types of other construction roads
  • Render minor construction=* types later
  • Reduce width for narrow highway=construction, to match finished road times (or narrower)
  • Add paths, track and null construction name label rendering
  • Change color of construction=null and construction=path/track to #aaa, medium gray (same as minor highway=construction)

Explanation:
There are several issues with the current highway=construction rendering. This PR addresses several:

  1. Highways under construction (construction=) should be no wider than their finished counterparts (highway=) on a given zoom level.
  2. Highway=construction with construction=null or =road and =track, =footway, etc currently render with a dark blue-gray color, which can look quite similar to an intermittent stream or river
  3. Highway=construction with construction=road, =pedestrian, =living_street and =raceway currently render starting at z12.
  • In fact, all highway=construction without construction=* are rendering at this level, and all unspecified values of construction=* get rendered at z12".
  1. Name labels are not rendered for construction=track/path/

There are several issues that this PR does not address, which will be discussed in #3579

I previously attempted to match the width of all roads at all zoom levels to their construction=* counterpart. However, the wider major roads at low zoom levels do not look right. Extensive adjustments would be needed to the dash pattern length, casing, and layering.

Dashed white and colored lines are quite prominent. This is problem is reduced currently, because highway=construction is rendered much narrower than major highways. Highway=construction does not follow proper layering order. Link roads (eg construction=motorway_link) can render on top of construction=motorway, and minor highways sometimes render on to of major highways. The layering order will sometimes reverse between different zoom levels. This is much more apparent if the roads are significantly wider.

These issues would need to be addressed before trying to match the width to wider roads at high zoom levels. But I believe this PR is a good start.

Test rendering:

z12 Before
z12-construction-master

After
z12-construction-after

z13
Before
z13-construction-master
After
z13-construction-after

z14 Before
z14-construction-master
After
z14-construction-after

z15 Before
z15-construction-master
After
z15-construction-after

z16 Before
z16-construction-master
After
z16-construction-after

z17 Before
z17-construction-master
After
z17-construction-after

Link roads z16 before
z16-construction-link-master
After
z16-construction-links-after

  • construction=secondary_link is now slightly narrower at this zoom level, to match the finished road.

@jeisenbe
Copy link
Collaborator Author

Here is a zip of the JOSM file (.osm.xml) that I made for testing. It includes all rendered types of construction=* and highway=* at different scales. There is also an area with construction=*_link roads, and areas of different difficult landcover colors for comparison to the highways:

test2.osm.xml.zip

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 22, 2018

After checking some more areas of highway=construction, I realized that highway=construction without construction=* (i.e. construction is null) should render the same as construction = residential and unclassified, rather than as construction=road. The majority of roads are highway=residential or highway=unclassified (which render the same), so it makes sense to assume that highway=construction is one of these, if not otherwise specified. I've pushed a commit to fix this.

Singapore, http://openstreetmap.org/#map=15/1.3862/103.9276
z16 master
z16-lorong-halus-master
z16 previous commit
z16-lorong-halus-current
z16 new commit
z16-lorong-halus-new

z15 Master
z15-lorong-halus-construction-roads-master
z15 Previous commit
z15-lorong-halus-construction-current
z15 New commit
z15-lorong-halus-new

z14 Master
z14-lorong-halus-master
z14 Previous commit
z13-lorong-halus-current
z14 New commit
z14-lorong-halus-new

@jragusa
Copy link
Contributor

jragusa commented Dec 22, 2018

Why the bridge is in motorway colour ?

BTW, could you also add rendering of bridge (#398) and tunnel (#761) for construction roads or is it better to do it in separate PRs ?

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 22, 2018 via email

@sommerluk
Copy link
Collaborator

Nice to see a PR for this issue. Better rendering for construction roads would be great!

I would keep the difference between “road” and “residential”. What is rendered for construction should follow the rendering for existing roads – yet for consistency.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 22, 2018 via email

@Tomasz-W
Copy link

Tomasz-W commented Dec 23, 2018

@jeisenbe Maybe a little bit off-topic, but do you have any idea for possible rail/ tram/ subway under construction rendering avoiding current dot-rendering of them? Otherwise it propably will be colliding with planned grey borders (see #2872 (comment))

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 23, 2018 via email

@Tomasz-W
Copy link

@jeisenbe If you figure something out, you can post it in #2872

Fixed link roads; remove unsupported construction=* types
Also added raceway, pedestrian, living_street, road as types of other construction roads
Reduce width for narrow construction highway
Add paths construction name label rendering
Add construction=track name label rendering
Add name label for construciton=null
Adjust width of construction=secondary_link at z16
Highway=construction with construction=null was set to render the same as construction=road; thinner and later than construction=residential
and construction=classified. Since highway=residential (and the identical highway=unclassified) are by far the most common type of road,
it is proposed to render highway=construction at the same levels and width if there is no construciton=* tag
@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 27, 2018 via email

@sommerluk
Copy link
Collaborator

The current PR does render construction=road and construction=residential differently, as shown in the test renderings above. The change was for highway=construction without any additional tags.

Okay, got it.

highway=construction without any additional tags should render like highway=construction with construction=road.

The reason is that currently we avoid yet to make a “best guess” in the code for road rendering. Example: For normal roads (no construction) with highway=road, we render them differently from residential or unclassified. Indeed we could also just “guess” that it is likely that they are residential or unclassified and render them like that. But we don’t because we want to give the mapper the feedback that he should use a more specific value than road.

Now, semantically, highway=construction without any additional tags is the same thing as highway=construction with construction=road: In both cases, there is a lack of information about the road type.

So, for reasons of consistency, highway=construction without any additional tags should render like highway=construction with construction=road.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Dec 27, 2018 via email

@sommerluk
Copy link
Collaborator

many mappers use just highway=construction without additional classification. This is relatively much more common than highway=road.

Maybe, but that’s not a good argument to treat it differently. Especially as your PR provides the much nicer rendering for these cases as the current one.

@dieterdreist
Copy link

dieterdreist commented Dec 29, 2018 via email

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 2, 2019

@matthijsmelissen could you take a look at this one? I believe it is ready to merge as-is, but I'd also be happy to change the rendering for generic highway=construction if needed.

@matthijsmelissen matthijsmelissen merged commit 6d560f7 into gravitystorm:master Jan 3, 2019
@matthijsmelissen
Copy link
Collaborator

Thanks, great work!

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 3, 2019

many mappers use just highway=construction without additional classification. This is relatively much more common than highway=road.

Maybe, but that’s not a good argument to treat it differently. Especially as your PR provides the much nicer rendering for these cases as the current one.

I don't quite understand this comment, I don't think the new rendering is "much nicer", though it is a minor improvement.

Currently highway=construction is rendered the same as construction=residential, and it uses a blue-gray dash pattern. This PR will change both highway=construction and construction=residential to render a little thinner at z14, z15 and z16 to match highway=residential, but the width is unchanged at <= z13 and >= z17. The color of the dashes is changed from blue-gray to neutral gray.

But I would be very happy to make more extensive changes to the current highway=construction rendering, in a future PR, since there are still outstanding issues with this rendering. Please comment on #3579

@jeisenbe jeisenbe deleted the construction branch January 3, 2019 23:20
@sommerluk
Copy link
Collaborator

I don't think the new rendering is "much nicer", though it is a minor improvement.

Well, you’ve changed the colour from blue-ish to grey-ish, and that’s good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants